[rfw] Fix fontWeight not applied in release mode#11581
[rfw] Fix fontWeight not applied in release mode#11581iam-abhijha wants to merge 2 commits intoflutter:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a dedicated fontWeight decoder to handle FontWeight values as strings or integers, ensuring stability in release mode. It updates strutStyle and textStyle to utilize this new decoder and includes comprehensive tests. Feedback identifies several instances where absolute keys are used for property lookups—specifically textDirection in the sweep shader, and textBaseline and overflow in textStyle—instead of relative keys, which prevents correct resolution when these structures are nested.
iam-abhijha
force-pushed
the
fix/rfw-font-weight-release-mode
branch
2 times, most recently
from
d9c573b to
a8bd4b5
Compare
|
Thanks for the contribution! In the future, please do not delete the checklist that is in the PR template; it is there for a reason. This PR is missing required elements described in the checklist, which need to be addressed before it moves forward with review. I am marking the PR as a Draft. Please re-add and complete the checklist, updating the PR as appropriate, and when that’s complete please feel free to mark the PR as ready for review. |
|
Thanks for the feedback! I've re added the PR template checklist and completed all applicable items. I have marked PR as Ready for review. Also this is my first time contributing to open source, so I appreciate the guidance! |
There was a problem hiding this comment.
Code Review
This pull request introduces a dedicated fontWeight decoder to resolve issues with FontWeight stability in release mode, updating TextStyle and StrutStyle to utilize it. It also fixes a key resolution bug in the sweep shader decoder and includes comprehensive unit tests for the new decoding logic. Feedback suggests simplifying the numeric FontWeight mapping by leveraging the FontWeight.values list for improved conciseness and robustness.
|
It looks like you've run the autoformatter on some files that don't use it (look for the |
iam-abhijha
force-pushed
the
fix/rfw-font-weight-release-mode
branch
2 times, most recently
from
524ee17 to
1944db2
Compare
|
Apologies for that, the auto-formatter was mistakenly run on the hand-formatted files. I've reverted all unrelated formatting changes; the PR now only touches argument_decoders.dart (new fontWeight decoder + two call site updates), argument_decoders_test.dart (regression test), CHANGELOG.md, and pubspec.yaml. |
iam-abhijha
force-pushed
the
fix/rfw-font-weight-release-mode
branch
from
f919678 to
359bc88
Compare
Please don't just repost AI-generated comments verbatim; reviewers don't need to have their review feedback repeated back to them. |
iam-abhijha
force-pushed
the
fix/rfw-font-weight-release-mode
branch
2 times, most recently
from
0f0019b to
869467b
Compare
FontWeight is not a real Dart enum; it is a hand-crafted class with
static const members. The previous implementation decoded fontWeight via
enumValue(), which matched strings against FontWeight.value.toString().
In AOT/release builds this is not guaranteed to be stable, so matches
silently failed and fontWeight defaulted to null (rendered as w400).
Add a dedicated ArgumentDecoders.fontWeight() method that maps strings
("w100"-"w900", "normal", "bold") and integers (100-900) to the correct
FontWeight constant using an explicit switch, without relying on
toString(). Update textStyle and strutStyle to use it.
Also fix three pre-existing bugs where absolute keys were used instead
of relative keys: textDirection in the sweep shader decoder, textBaseline
and overflow in textStyle.
Fixes flutter/flutter#180223
Made-with: Cursor
Made-with: Cursor
iam-abhijha
force-pushed
the
fix/rfw-font-weight-release-mode
branch
from
869467b to
a41ff8d
Compare
Sorry about that It's my first pr not much aware of rules. |
2 participants
Fixes the
fontWeightproperty of RFWTextStyle(andStrutStyle) not being applied in release mode forTextwidgets.Fixes flutter/flutter#180223
Root cause
ArgumentDecoders.textStyle(andstrutStyle) decodedfontWeightusingenumValue(FontWeight.values, ...), which matched the input string againstFontWeight.value.toString().split('.').last.FontWeightis not a real Dartenum; it is a hand-crafted class withstatic constmembers. In AOT/release builds,toString()for such classes is not guaranteed to return the same human-readable name (tree-shaking can strip debug labels), so all matches silently fail andnullis returned, causing Flutter to fall back to the default weight (w400).Fix
Added a new
ArgumentDecoders.fontWeightstatic method that maps strings ("w100"–"w900","normal","bold") and integers (100–900) to the correctFontWeightconstant using an explicitswitch, without relying ontoString(). Updated bothtextStyleandstrutStyleto use this new decoder.Pre-Review Checklist
[shared_preferences]///).